Fix InvalidMemoryOperationError in DebugAllocator #848
Conversation
|
Hm, but like this it doesn't make sense. It now tries to remove a "pointer to a pointer to void" from a set of "pointers to void". Do you have a call stack for the invalid memory operation error? |
|
I couldn't get a call stack no, but I traced it using log trace. It occurs somewhere around http/router.d:320 |
|
I'll see if I can reproduce it. But probably I won't be able to make it before Saturday, as I need to prepare for a journey. |
|
Oh, just uncomment memory.d:46 and run dub test. That's alright, it's only annoying not knowing where it comes from the first time it happens. You're right though, my fix only works because it doesn't remove anything. Btw, the pointer it tries to remove looks really odd |
|
Actually not that odd, it's 23C1F00 and 23D6550, using try/catch does the thing on linux. |
|
I figured out a way to get a stack trace (calling Program received signal SIGTERM, Terminated. |
|
Oh, I debugged this already for LDC. I thought it was compiler specific: https://github.com/etcimon/libasync/blob/master/source/libasync/internals/memory.d#L654 |
|
Now I'm worried that it may leave entries in DebugAllocator. I'll get back to it later |
|
Ah okay... so it's because the |
|
Oh, that's a great solution. Plus, I've gotten used to using the |
| @@ -141,9 +142,10 @@ final class DebugAllocator : Allocator { | |||
| this(Allocator base_allocator) | |||
| { | |||
| m_baseAlloc = base_allocator; | |||
| m_blocks = new HashMap!(void*, size_t)(manualAllocator()); | |||
There was a problem hiding this comment.
Any reason for the new? Couldn't we just use a plain HashMap here instead of a pointer?
| @@ -648,7 +650,7 @@ struct FreeListRef(T, bool INIT = true) | |||
| //logInfo("ref %s destroyed", T.stringof); | |||
| } | |||
| static if( hasIndirections!T ) GC.removeRange(cast(void*)m_object); | |||
| manualAllocator().free((cast(void*)m_object)[0 .. ElemSize+int.sizeof]); | |||
| static if (!INIT) manualAllocator().free((cast(void*)m_object)[0 .. ElemSize+int.sizeof]); | |||
There was a problem hiding this comment.
Why did you add this static if, did this fix some kind of crash? If I'm not completely mistaken, this will create a memory leak for objects with INIT == true, destroy itself doesn't do any deallocation.
There was a problem hiding this comment.
Yes, I saw a crash where there was an invalid memory operation because the .destroy seemed to have already gotten rid of the object. However, I can't remember how or when it had occurred, should be in the HTTPServer if I'm not mistaken. It was also back when I was testing LDC..
It looks like it's around the same time: ldc-developers/ldc#729
There was a problem hiding this comment.
Yes this is probably part of an interface issue in LDC. I'll remove it
Remove bad formatting
|
Okay thanks. I'll merge now.
Do you mean |
Fix InvalidMemoryOperationError in DebugAllocator
|
I had an |
|
Hm, maybe it was just the initial workaround for the memory operation error in |
Could be. The interface problems are still there without the debug allocator though, I got ldc and llvm loaded into clion in an attempt to look for a clue about it. Is vibe.d supposed to be compatible with ldc? |
|
Some reports had been positive, but I didn't yet successfully test it on LDC. In fact it crashes with a stack trace when I try to compile the |
It's going to take a few design changes in LDC I think, I've been reading the code for clang to get familiar with the LLVM way and I expect to get involved with that in 3-4 months. The LDC compiler was one of the reasons I started coding with D, it's the only way D comes on top of C and C++ while offering the pleasant high-level language facilities. |
This fixes errors which fail unit tests when using DebugAllocator.